-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Actions] System actions enhancements #161340
Conversation
…ana into system_actions_registration
@@ -119,14 +125,17 @@ export async function getInUseTotalCount( | |||
countEmailByService: Record<string, number>; | |||
countNamespaces: number; | |||
}> { | |||
const scriptedMetric = { | |||
const getInMemoryActionScriptedMetric = (actionRefPrefix: string) => ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I switch the order of scriptedMetric
and preconfiguredActionsScriptedMetric
(which I made it a function)
const inMemoryConnectors = getInMemoryConnectors().filter( | ||
(inMemoryConnector) => inMemoryConnector.isPreconfigured | ||
); | ||
const inMemoryConnectors = getInMemoryConnectors(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will take system actions into account in telemetry.
6009fd6
to
25a93f0
Compare
Pinging @elastic/response-ops (Team:ResponseOps) |
@elasticmachine merge upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM! Left a few nits and questions.
(!actionTypeEnabled && | ||
this.inMemoryConnectors.find((inMemoryConnector) => inMemoryConnector.id === actionId) !== | ||
undefined) | ||
(!actionTypeEnabled && inMemoryConnector !== undefined && inMemoryConnector.isPreconfigured) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional nit:
(!actionTypeEnabled && inMemoryConnector !== undefined && inMemoryConnector.isPreconfigured) | |
inMemoryConnector?.isPreconfigured === true |
* Returns true if action type is enabled or it is a preconfigured action type | ||
* It is possible for to disable an action type but use a preconfigured action | ||
* of action type the disabled one. This does not apply to system actions. | ||
* It should be possible to disable a system action type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Returns true if action type is enabled or it is a preconfigured action type | |
* It is possible for to disable an action type but use a preconfigured action | |
* of action type the disabled one. This does not apply to system actions. | |
* It should be possible to disable a system action type. | |
* Returns true if action type is enabled or preconfigured. | |
* An action type can be disabled but used with a preconfigured action. | |
* This does not apply to system actions as those can be disabled. |
(!actionTypeEnabled && | ||
this.inMemoryConnectors.find((inMemoryConnector) => inMemoryConnector.id === actionId) !== | ||
undefined) | ||
(!actionTypeEnabled && inMemoryConnector !== undefined && inMemoryConnector.isPreconfigured) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, we should see with @shanisagiv1 if we wan the capability of disabling system actions or not. We can start with the context of the case action (and maybe think of future ones too).
If we are unsure, maybe it's best to hold off in case configuring via kibana.yml
isn't the right answer? (ex: what about feature controls, serverless, etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed offline and we decided to let the system actions to be disabled from the kibana.yml
const preConfiguredConnectors = this.inMemoryConnectors.filter( | ||
(connector) => connector.isPreconfigured | ||
); | ||
|
||
const isSystemActionAsPreconfiguredInConfig = preConfiguredConnectors.some((connector) => | ||
this.actionTypeRegistry!.isSystemActionType(connector.actionTypeId) | ||
); | ||
|
||
if (isSystemActionAsPreconfiguredInConfig) { | ||
throw new Error('Setting system action types in preconfigured connectors are not allowed'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional nit: in case you want to condense the code
const preConfiguredConnectors = this.inMemoryConnectors.filter( | |
(connector) => connector.isPreconfigured | |
); | |
const isSystemActionAsPreconfiguredInConfig = preConfiguredConnectors.some((connector) => | |
this.actionTypeRegistry!.isSystemActionType(connector.actionTypeId) | |
); | |
if (isSystemActionAsPreconfiguredInConfig) { | |
throw new Error('Setting system action types in preconfigured connectors are not allowed'); | |
} | |
const hasSystemActionAsPreconfiguredInConfig = this.inMemoryConnectors | |
.filter((connector) => connector.isPreconfigured) | |
.some((connector) => this.actionTypeRegistry!.isSystemActionType(connector.actionTypeId)); | |
if (hasSystemActionAsPreconfiguredInConfig) { | |
throw new Error('Setting system action types in preconfigured connectors are not allowed'); | |
} |
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: cc @cnasikas |
## Summary This PR: - Handles the references for system actions in the rule - Forbids the creation of system actions through the `kibana.yml` - Adds telemetry for system actions - Allow system action types to be disabled from the `kibana.yml` Depends on: elastic#160983, elastic#161341 ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
This PR:
kibana.yml
kibana.yml
Depends on: #160983, #161341
Checklist
Delete any items that are not applicable to this PR.
For maintainers